Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change docstring style to numpydocs #901

Merged
merged 43 commits into from
Sep 23, 2024
Merged

Change docstring style to numpydocs #901

merged 43 commits into from
Sep 23, 2024

Conversation

Opt-Mucca
Copy link
Collaborator

What was done in this MR:

  • Unified docstring style to numpydoc
  • Added missing information on parameters to docstring where appropriate
  • Add new style of showing the API to the online documentation.

What I did not do:

  • Change the docstrings of all tests + other pieces of code. I think much of this will come from implementing a style formatter.
  • I did not add type hints. If anything I removed the few that we had. Guides online suggest that they are redundant if the docstrings are done properly. IDEs should now be able to extract the types for all parameters for instance. Implementing type hints in Cython is also different than standard Python. For instance, mypy requires an additional file with each function declaration.

Joao-Dionisio
Joao-Dionisio requested changes Sep 20, 2024
View reviewed changes
Copy link
Collaborator

@Joao-Dionisio Joao-Dionisio left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My god Mark. I don't even know what to say. Just reviewing this took a bunch of time, I can't imagine what it was like for you ahah. Just these minor typos, most of which were already there

I'll commit the suggestions that are clearly correct.

src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved
src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved
src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved
src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved
src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved
src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved
src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved
src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved
src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved
src/pyscipopt/scip.pxi Outdated Show resolved Hide resolved
Opt-Mucca and others added 23 commits September 23, 2024 11:31
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
9e7b24f 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
080ce76 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
3a307a9 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
e2d3bfc 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
fc4e587 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
526932c 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
a3fcc15 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
759ce86 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
5cc3e0f 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
b62f553 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
ac58dac 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
bc96bd8 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
ed74fdc 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
49f8ffe 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
63d656d 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
5d02a44 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
f97ddeb 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
40bca6e 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
e0148ab 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
ff0f698 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
22f9eef 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca
Update some comments from Joao
4766d58
@Opt-Mucca @Joao-Dionisio
Update src/pyscipopt/scip.pxi
90a89e7 
Co-authored-by: João Dionísio <[email protected]>
@Opt-Mucca
Copy link
Collaborator Author

@Joao-Dionisio I didn't expect such a robust review. So I'm not sure if I even spent that much more time comparatively.
All the changes should be addressed now! Thank you for fixing a lot of the small errors.

@Joao-Dionisio Joao-Dionisio merged commit 729420c into master Sep 23, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Joao-Dionisio Joao-Dionisio Joao-Dionisio approved these changes

@mmghannam mmghannam Awaiting requested review from mmghannam

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Opt-Mucca @mmghannam @Joao-Dionisio